Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Windows CI job #218

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add Windows CI job #218

wants to merge 2 commits into from

Conversation

pka
Copy link
Member

@pka pka commented Sep 15, 2021

  • I agree to follow the project's code of conduct.
  • I added an entry to CHANGES.md if knowledge of this change could be valuable to users.

Test failures are ignored for now, because we have a few failures which are tracked in #219.

@rmanoka
Copy link
Contributor

rmanoka commented Sep 16, 2021

Nice work @pka ; here's what I found about the failures:

  1. raster::tests::test_create_with_band_type_with_options : we're trying to create /tmp/test.tif which is not valid path in windows. Should use a proper env::temp_dir for platform compat.
  2. vector::vector_tests::tests::test_write_features : seems to be from fs::remove_file when the dataset is open. Should remove the file after the dataset is dropped I guess.
  3. spatial_ref::tests::auto_identify: the error code is OGRERR_UNSUPPORTED_SRS so I suppose the GDAL installation is outdated / doesn't contain many coordinate systems?

@pka pka mentioned this pull request Sep 18, 2021
@pka
Copy link
Member Author

pka commented Sep 18, 2021

Made a separate issue for the test failures: #219

@rmanoka
Copy link
Contributor

rmanoka commented Sep 22, 2021

Looks good @pka ; we could handle the errors in a separate PR. Should we merge this as is?

@pka
Copy link
Member Author

pka commented Sep 22, 2021

I don't plan to fix the Windows tests bugs within this PR. When they are fixed, the || true can be removed. It's already useful as it is, since it tests the Windows build.

@lnicola
Copy link
Member

lnicola commented Sep 22, 2021

Just curious, why the -j1?

@pka
Copy link
Member Author

pka commented Sep 22, 2021

There is a impressive collection of build failures in https://github.com/pka/rust-gdal/actions. cargo build -j1 fixed most of them. My guess is, that bindgen has to be built early, otherwise other crates fail to build.

@lnicola
Copy link
Member

lnicola commented Sep 22, 2021

thread 'main' panicked at 'Unable to find libclang: "the `libclang` shared library at C:\\msys64\\mingw64\\bin\\libclang.dll could not be opened: LoadLibraryExW failed"', C:\Users\runneradmin\.cargo\registry\src\github.com-1ecc6299db9ec823\bindgen-0.59.1\src/lib.rs:2117:31

😕

@pka
Copy link
Member Author

pka commented Sep 22, 2021

Correction: The main failure -j1 solved was

thread 'main' panicked at 'Could not find `pkg-config` in your path. Please install it before building gdal-sys.', gdal-sys\build.rs:229:17

The libclang.dll could not be opened: LoadLibraryExW failed error was solved with prepending /c/msys64/mingw64/bin to $PATH.

@pka
Copy link
Member Author

pka commented Dec 2, 2021

In the meantime, the sdk from gisinternals has GDAL 3.4. So #231 has to be merged first.

@jdroenner
Copy link
Member

@pka what is the state of this PR? Is there something missing?

@pka
Copy link
Member Author

pka commented May 20, 2022

It was ready for merge in January 22, but didn't test it again in the meantime. I'll do a rebase and we'll see if the build is still working.

@jdroenner
Copy link
Member

great! if all tests are green i will merge it.

@pka
Copy link
Member Author

pka commented May 20, 2022

More failing tests on Windows in the meantime (see #219), but build is working.

@pka
Copy link
Member Author

pka commented May 23, 2022

Incompatible tests are now disabled and CI doesn't ignore failing tests anymore.

@pka pka mentioned this pull request Nov 21, 2022
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants